Conversation
Signed-off-by: Cassandra Coyle <cassie@diagrid.io>
There was a problem hiding this comment.
Pull request overview
Removes the SDK’s implicit 60s gRPC deadline so long-running operations (e.g., workflows/LLM calls) aren’t cut off before Dapr resiliency policies and component timeouts can apply.
Changes:
- Set
DAPR_API_TIMEOUT_SECONDSdefault toNone(no deadline unless explicitly configured). - Update sync and async gRPC timeout interceptors to only apply a deadline when
DAPR_API_TIMEOUT_SECONDSis configured, and to pass it as afloat.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
dapr/conf/global_settings.py |
Removes the hardcoded default gRPC timeout by setting DAPR_API_TIMEOUT_SECONDS to None. |
dapr/clients/grpc/interceptors.py |
Applies a global gRPC timeout only when explicitly configured via settings. |
dapr/aio/clients/grpc/interceptors.py |
Mirrors the explicit-only timeout behavior for the asyncio gRPC client. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Signed-off-by: Cassandra Coyle <cassie@diagrid.io>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #956 +/- ##
==========================================
+ Coverage 86.63% 89.06% +2.43%
==========================================
Files 84 104 +20
Lines 4473 7410 +2937
==========================================
+ Hits 3875 6600 +2725
- Misses 598 810 +212 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| self.assertEqual(7, called_client_call_details.timeout) | ||
|
|
||
| @patch.object(settings, 'DAPR_API_TIMEOUT_SECONDS', None) | ||
| def test_intercept_unary_unary_no_global_timeout_no_per_call_timeout(self): |
There was a problem hiding this comment.
Don't we need a test that covers DAPR_API_TIMEOUT_SECONDS being None and timeout having a value?
Remove hardcoded 60s default timeout. Only apply if explicitly configured.
I was running a long running agent/wf and it was being killed by this and it was not respecting the resiliency configured. An SDK-level default deadline that fires before resiliency can act prevents retries from ever being attempted...